Skip to content

Conversation

@elsakeirouz
Copy link

Most variadic generic functions contain loops that iterate over the elements of their Pack parameters. Since the types of the pack elements are unknown, witness table methods are used to perform operations on the current pack element in each iteration. After specialization, these loops are usually unrolled, since there is a fixed list of pack element types. In the unrolled code, it should be possible to eliminate the witness table lookups. This is not possible because of the @pack_element types introduced to refer to pack elements:

%5 = integer_literal $Builtin.Word, 0
%6 = dynamic_pack_index %5 of $Pack{Int, Double}
%7 = open_pack_element %6 of at <Pack{Int, Double}>, shape $each A, uuid "063BDCF4-98A3-11F0-B2E8-929C59BC796C"
%20 = witness_method $@pack_element("063BDCF4-98A3-11F0-B2E8-929C59BC796C") each A, #Numeric."*" …
%21 = apply %20<@pack_element("063BDCF4-98A3-11F0-B2E8-929C59BC796C") each A>(%10, %14, %18, %11) … // type-defs: %7

This simplification aims to eliminate all instances of @pack_element types from specialized functions by replacing those with the underlying concrete type. This allows the appropriate witness method, as well as other functions, to be inlined.

This computed attribute is used in the simplifications of witness_method, alloc_stack, unchecked_addr_cast and apply.

@elsakeirouz elsakeirouz self-assigned this Nov 5, 2025
@eeckstein eeckstein requested a review from rjmccall November 6, 2025 07:49
@elsakeirouz elsakeirouz force-pushed the eliminate-pack_element-types branch 4 times, most recently from 4db84e1 to ae181f8 Compare November 6, 2025 18:11
* SwiftCompilerSources/Sources/AST/Type.swift
* SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyAllocStack.swift
* SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyUncheckedAddrCast.swift
* SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyWitnessMethod.swift
* SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift
* include/swift/AST/ASTBridging.h
* include/swift/AST/ASTBridgingImpl.h
@elsakeirouz elsakeirouz force-pushed the eliminate-pack_element-types branch 2 times, most recently from 491bbf8 to 71d3673 Compare November 6, 2025 18:12
Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Can we reasonably just ask SILCloner to canonicalize this, so that we don't end up with pack_element archetypes that can be resolved to a concrete type in the first place? Otherwise I'm concerned that we're going to play whack-a-mole with every instruction that takes a type operand.

I think this can be done elegantly with a combination of

  1. specializing the cloning of DynamicPackIndexInst to turn it into a ScalarPackIndexInst when the index value is an integer_literal and there's a prefix of non-expansion element types in the pack (which I don't think you're checking for), and
  2. specializing the cloning of OpenPackElementInst when the index is a ScalarPackIndexInst.

The result of OpenPackElementInst is only used as a type-dependent operand for uses of the local archetype it produces, so it should be fine to simply not create a corresponding value in the clone.

You may also need to do some similar canonicalization with PackPackIndexInst; the test case is a variadic generic function that creates a larger pack, e.g.

func foo<each T>(values: repeat each T) {}
func bar<each T>(values: repeat each T) {
  foo(values: repeat each values, 1, 2, 3)
}

@eeckstein
Copy link
Contributor

Can we reasonably just ask SILCloner to canonicalize this

That's an interesting idea. However, when the cloner runs to specialize the pack for concrete types, the pack loop is still not unrolled and pack indices are not constant folded, etc. So it cannot see the constant element type.
Also, I'm not a big fan of adding peephole optimization logic to such a fundamental tool like the SIL cloner.
I think it's fine to handle the few kind of relevant instructions in simplification passes (we do a similar thing for existential archetypes). We'll add swift source -> SIL end-to-end tests anyway. So we'll catch missed optimization opportunities.

... there's a prefix of non-expansion element types in the pack (which I don't think you're checking for)

Good catch! This check needs to be added

@rjmccall
Copy link
Contributor

rjmccall commented Nov 7, 2025

Can we reasonably just ask SILCloner to canonicalize this

That's an interesting idea. However, when the cloner runs to specialize the pack for concrete types, the pack loop is still not unrolled and pack indices are not constant folded, etc. So it cannot see the constant element type.

Unrolling the loop is presumably also going to be done by the SILCloner.

Also, I'm not a big fan of adding peephole optimization logic to such a fundamental tool like the SIL cloner.

Okay, then add it as a specific override in the cloner you use to unroll the pack loop. Or if you really want to do this retroactively instead of while you're unrolling (which is going to mean cloning almost all the instructions in the loop body twice per unroll, but okay), you should just have the optimizer recognize that you can fold an open_pack_element to a concrete type and then use a type-substituting cloner (which could be the same cloner as for existentials) to rewrite in place the entire tree of dependent instructions that refer to the pack element archetype.

The point is that this is just a type substitution. We have a standard code path for rewriting every kind of instruction under a type substitution, which is the SIL cloner, and we should not be making redundant partial copies of that logic.

I think it's fine to handle the few kind of relevant instructions in simplification passes

There are more than a few relevant instructions, though. Almost every use of the pack element would benefit from knowing that it's a specific type. The goal of this optimization should be to eliminate the pack element archetype entirely, and the most reasonable way to do that is with a SIL cloner that has the effect of replacing the pack element archetype with the concrete pack element type in the unrolled loop body.

(we do a similar thing for existential archetypes).

The existential archetype specialization code should also be using the SIL cloner to substitute the existential archetype with the erased type, rather than hard-coding that for specific instructions.

@eeckstein
Copy link
Contributor

and then use a type-substituting cloner (which could be the same cloner as for existentials) to rewrite in place the entire tree of dependent instructions that refer to the pack element archetype.

That's a reasonable approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants